circular resume touch fixes #2022
Conversation
| android:layout_gravity="center" | ||
| android:layout_marginStart="10dp" | ||
| android:gravity="center" | ||
| android:letterSpacing="0.09" |
| android:layout_marginStart="10dp" | ||
| android:gravity="center" | ||
| android:letterSpacing="0.09" | ||
| android:textColor="?attr/black" |
There was a problem hiding this comment.
Why? This does not match the other buttons color at all.
| tools:ignore="RtlSymmetry" | ||
| android:letterSpacing="0.09" | ||
| android:textAllCaps="false" | ||
| android:textColor="?attr/black" |
| tools:text="69m remaining" | ||
| android:textStyle="bold" | ||
| tools:ignore="RtlSymmetry" | ||
| android:letterSpacing="0.09" |
| observeNullable(viewModel.movie) { data -> | ||
| resultBinding?.apply { | ||
| resultPlayMovie.isVisible = data is Resource.Success | ||
| resultPlayMovie.isVisible = data is Resource.Success && (viewModel.resumeWatching.value == null) |
There was a problem hiding this comment.
This looks like it could be a race condition. Please do not do logic that depends on two different values.
|
@fire-light42 I think previously fixes are not that good. Making clickable and focusable to false solved click issue i didn't noticed that before, and moving watching observable below movie solved play movie visibility issue. |
|
|
||
| (data as? Resource.Success)?.value?.let { (text, ep) -> | ||
| resultPlayMovie.setText(text) | ||
| // resultPlayMovie.setText(text) |
There was a problem hiding this comment.
I know that setText may change visibility here, but it is required that the text of resultPlayMovie is set as otherwise it will say "Play Movie" on both torrents and livestreams.
There was a problem hiding this comment.
You could manually do the .toStringNull here if visibility is not what you want.
| ) | ||
| return@setOnLongClickListener true | ||
| } | ||
| resultResumeSeriesButton.setOnLongClickListener { |
There was a problem hiding this comment.
I am unable to trigger this?
There was a problem hiding this comment.
Did you mean to place this somewhere else given that no series will be a valid movie?
There was a problem hiding this comment.
Yeah it's only for movie
That screams race condition to me, do not rely upon the execution order of observe. Observe may trigger in any order, so place |
rockhero1234
force-pushed
the
uplift_circular
branch
from
da98f6f to
4ee75ec
Compare
|
@fire-light42 did changes |
2 participants
No description provided.